Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(common): adding getSession method #160

Merged
merged 1 commit into from
Aug 19, 2024

Conversation

aversini
Copy link
Collaborator

@aversini aversini commented Aug 19, 2024

PR Type

enhancement, tests


Description

  • Implemented the getSession function to extract session IDs from request headers, specifically cookies.
  • Added unit tests for the getSession function to ensure correct functionality and edge case handling.
  • Updated module exports to include the new getSession function.

Changes walkthrough 📝

Relevant files
Tests
getSession.test.ts
Add unit tests for getSession function                                     

packages/auth-common/src/components/tests/getSession.test.ts

  • Added unit tests for getSession function.
  • Tested scenarios for session extraction from cookies.
  • Included test for missing session in cookies.
  • +25/-0   
    Enhancement
    getSession.ts
    Implement getSession function to extract session ID           

    packages/auth-common/src/components/getSession.ts

  • Implemented getSession function to extract session ID from cookies.
  • Added helper function getFromCookie for session extraction.
  • Defined GetSessionProps type for function parameters.
  • +31/-0   
    index.ts
    Export getSession function in module index                             

    packages/auth-common/src/components/index.ts

    • Exported getSession function from the module.
    +1/-0     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 PR contains tests
    🔒 Security concerns

    Regular expression vulnerability:
    The regular expression used in the getFromCookie function (line 8-9 in getSession.ts) might be vulnerable to ReDoS attacks if not properly bounded. Consider using a more specific pattern or implementing a timeout mechanism to prevent potential abuse.

    ⚡ Key issues to review

    Potential Security Issue
    The regular expression used for extracting the session from the cookie might be vulnerable to ReDoS (Regular Expression Denial of Service) attacks if not properly bounded.

    Error Handling
    The function doesn't handle potential errors, such as invalid cookie format or unexpected input types.

    Copy link

    Bundle Size

    Status File Size (Gzip) Limits
    index.js 17.81 KB (+9 B +0.05%) 18 kb

    Overall bundle size: 17.81 KB (+9 B +0.05%)
    Overall status: ✅

    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Add test cases for undefined and null headers to ensure proper handling of edge cases

    Add a test case for when the headers object is undefined or null. This will ensure
    that the getSession function handles these edge cases correctly.

    packages/auth-common/src/components/tests/getSession.test.ts [3-9]

     describe("getSession", () => {
       const clientId = "testClient";
     
       it("should return empty string if no session is found", () => {
         const headers: HeadersLike = {};
         expect(getSession({ headers, clientId })).toBe("");
       });
     
    +  it("should handle undefined headers", () => {
    +    expect(getSession({ headers: undefined, clientId })).toBe("");
    +  });
    +
    +  it("should handle null headers", () => {
    +    expect(getSession({ headers: null, clientId })).toBe("");
    +  });
    +
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: Adding these test cases is crucial for ensuring the robustness of the function by verifying that it correctly handles edge cases, which is a best practice in testing.

    9
    Enhancement
    Simplify the cookie parsing function using optional chaining and nullish coalescing operators

    The getFromCookie function could be improved by using optional chaining and nullish
    coalescing operators. This would make the code more concise and easier to read.

    packages/auth-common/src/components/getSession.ts [3-14]

     const getFromCookie = (headers: HeadersLike, clientId: string) => {
       const cookie = headers?.cookie;
    -  if (typeof cookie !== "string") {
    -    return;
    -  }
    +  if (typeof cookie !== "string") return null;
       const re = new RegExp(`auth.${clientId}.session=(.+?)(?:;|$)`);
    -  const match = cookie.match(re);
    -  if (!match) {
    -    return;
    -  }
    -  return match[1];
    +  return cookie.match(re)?.[1] ?? null;
     };
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion enhances code readability and conciseness by using modern JavaScript features, making the function easier to understand and maintain.

    8
    Change the return type to be more specific and return null instead of an empty string when no session is found

    Consider using a more specific return type for the getSession function. Instead of
    returning an empty string when no session is found, it might be more appropriate to
    return null or undefined. This would make it clearer that no session was found,
    rather than an empty session.

    packages/auth-common/src/components/getSession.ts [27-31]

    -export const getSession = ({ headers, clientId }: GetSessionProps): string => {
    +export const getSession = ({ headers, clientId }: GetSessionProps): string | null => {
       const fromCookie = getFromCookie(headers, clientId);
     
    -  return fromCookie || "";
    +  return fromCookie || null;
     };
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: This suggestion improves the clarity of the function's return value by distinguishing between an empty session and no session found, which can be beneficial for error handling and debugging.

    7
    Possible issue
    Add input validation for the clientId parameter to prevent potential issues with invalid input

    Consider adding input validation for the clientId parameter in the getSession
    function. This could prevent potential issues if an invalid clientId is provided.

    packages/auth-common/src/components/getSession.ts [27-31]

     export const getSession = ({ headers, clientId }: GetSessionProps): string => {
    +  if (!clientId || typeof clientId !== 'string') {
    +    throw new Error('Invalid clientId provided');
    +  }
       const fromCookie = getFromCookie(headers, clientId);
     
       return fromCookie || "";
     };
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Input validation is important for preventing potential runtime errors and ensuring that the function behaves predictably when given invalid input, enhancing the overall reliability of the code.

    8

    @aversini aversini merged commit 2454df2 into main Aug 19, 2024
    4 checks passed
    @aversini aversini deleted the feat(common)-adding-getSession-method branch August 19, 2024 15:55
    @aversini aversini mentioned this pull request Aug 19, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant